feat: add skill-load coverage for agent integrations [2/3]#392
Conversation
github-actions
Bot
added
size:XL
WalkthroughAdds tests confirming structured skill reads emit ChangesSkill-load mark validation
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
License DiffCompared against Lockfile license changesLockfile License ChangesRustAdded
Removed
Updated/Changed
NodeAdded
Removed
Updated/Changed
PythonAdded
Removed
Updated/Changed
Status output |
|
I’d drop the OpenClaw-specific skill-load handling from this stack. With the CLI/sidecar pivot, extending hook replay with eager/fallback state looks like unnecessary complexity. I’d keep the shared core and other integration changes, remove the corresponding OpenClaw changes from 392/393, and handle managed OpenClaw skill marks at the sidecar boundary if we still need them. That would also make my OpenClaw-specific inline comments moot. |
github-actions
Bot
removed
the
lang:js
willkill07
changed the title
willkill07
force-pushed
the
wkk_relay-443-integrations
branch
from
89db64d to
4a09ce7
Compare
Signed-off-by: Will Killian <wkillian@nvidia.com>
UserPromptExpansion is only present in Claude Code's plugin hook whitelist from 2.1.116. Older hosts reject the entire plugin hooks file on the unknown event name and silently load no relay hooks, for both the transparent-run temp plugin and the marketplace plugin. Document the verified floor on HOOK_EVENTS, make doctor warn when the probed Claude Code predates it, and state the requirement in the plugin README. Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com> (cherry picked from commit d3b8959)
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
willkill07
force-pushed
the
wkk_relay-443-integrations
branch
from
4a09ce7 to
a29a815
Compare
github-actions
Bot
added
size:M
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@python/tests/integrations/deepagents_tests/test_deepagents_integration.py`:
- Around line 332-369: Add a parent-scope-correlation assertion to
test_skill_load_mark_survives_deepagents_middleware: identify the read_file
start ScopeEvent from the captured lifecycle events and assert
marks[0].parent_uuid matches that event’s scope UUID, consistent with the
sibling LangChain and LangGraph tests.
In `@python/tests/integrations/langchain_tests/test_middleware.py`:
- Around line 414-427: Replace the manual subscriber lifecycle in the affected
test with the existing subscribed_events pytest fixture, as used by the
LangGraph and DeepAgents integration tests. Remove the explicit register, flush,
and deregister calls around wrap_tool_call, and assert against the fixture’s
collected events so all skill-load tests share the same setup.
In `@python/tests/test_tools.py`:
- Around line 71-78: Replace the manual events list, subscriber registration,
flush, and finally-based deregistration in the affected test with the shared
subscribed_events pytest fixture. Update assertions to read captured events from
that fixture, and apply the same fixture-based setup to the corresponding
LangChain middleware test to remove duplicated subscription boilerplate.
- Line 72: Replace the unnecessary lambda passed to subscribers.register in the
test with the existing events.append method directly, preserving the current
event collection behavior and matching the integration test pattern.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 0a02cd5e-285c-4ca7-bed9-dffa1af8b009
📒 Files selected for processing (4)
python/tests/integrations/deepagents_tests/test_deepagents_integration.pypython/tests/integrations/langchain_tests/test_middleware.pypython/tests/integrations/langgraph_tests/test_langgraph_integration.pypython/tests/test_tools.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: CodeRabbit
- GitHub Check: Check / Run
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
python/tests/test_tools.pypython/tests/integrations/langgraph_tests/test_langgraph_integration.pypython/tests/integrations/deepagents_tests/test_deepagents_integration.pypython/tests/integrations/langchain_tests/test_middleware.py
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
python/tests/test_tools.pypython/tests/integrations/langgraph_tests/test_langgraph_integration.pypython/tests/integrations/deepagents_tests/test_deepagents_integration.pypython/tests/integrations/langchain_tests/test_middleware.py
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: When changing the Python wrapper package, tests, or docs tooling, lint with Ruff (E,F,W,I), format with Ruff formatter (120-character lines, double quotes), and passtytype checking.
Add the SPDX license header to all Python source files using the#comment form.
Files:
python/tests/test_tools.pypython/tests/integrations/langgraph_tests/test_langgraph_integration.pypython/tests/integrations/deepagents_tests/test_deepagents_integration.pypython/tests/integrations/langchain_tests/test_middleware.py
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
python/tests/test_tools.pypython/tests/integrations/langgraph_tests/test_langgraph_integration.pypython/tests/integrations/deepagents_tests/test_deepagents_integration.pypython/tests/integrations/langchain_tests/test_middleware.py
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
python/tests/test_tools.pypython/tests/integrations/langgraph_tests/test_langgraph_integration.pypython/tests/integrations/deepagents_tests/test_deepagents_integration.pypython/tests/integrations/langchain_tests/test_middleware.py
**/*.{py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Keep Python, Go, and Node.js config objects and subscriber/exporter methods aligned so all bindings expose the same logical knobs and semantics.
Files:
python/tests/test_tools.pypython/tests/integrations/langgraph_tests/test_langgraph_integration.pypython/tests/integrations/deepagents_tests/test_deepagents_integration.pypython/tests/integrations/langchain_tests/test_middleware.py
python/tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
python/tests/**/*.py: Pytest is used to run tests.
Do not add@pytest.mark.asyncioto any test; async tests are automatically detected and run by the async runner.
Do not add a-> Nonereturn type annotation to test functions.
When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, with thespecconstructor argument when necessary.
Name mocked classes with themockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; if a fixture is needed in multiple test files, place it in aconftest.pyfile.
When creating a fixture, use@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and define the fixture function asdef <fixture_name>_fixture() -> <return_type>:; only specifyscopewhen it is notfunction.
Preferpytest.mark.parametrizeover creating individual tests for different input types.
Files:
python/tests/test_tools.pypython/tests/integrations/langgraph_tests/test_langgraph_integration.pypython/tests/integrations/deepagents_tests/test_deepagents_integration.pypython/tests/integrations/langchain_tests/test_middleware.py
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
python/tests/test_tools.pypython/tests/integrations/langgraph_tests/test_langgraph_integration.pypython/tests/integrations/deepagents_tests/test_deepagents_integration.pypython/tests/integrations/langchain_tests/test_middleware.py
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
python/tests/test_tools.pypython/tests/integrations/langgraph_tests/test_langgraph_integration.pypython/tests/integrations/deepagents_tests/test_deepagents_integration.pypython/tests/integrations/langchain_tests/test_middleware.py
**
⚙️ CodeRabbit configuration file
**:AGENTS.md
This file provides guidance to agents, including Claude Code and OpenAI Codex, when working in this repository.
Project Overview
NeMo Relay is a multi-language agent runtime framework for execution scopes, lifecycle events, middleware, plugins, and observability around tool and LLM calls. The core runtime is Rust. Primary supported bindings are Rust, Python, and Node.js. Go and the raw C FFI are experimental and source-first.
The shared runtime model is:
- Scope stacks decide where work belongs and which scope-local behavior is visible.
- Middleware registries decide what guardrails and intercepts run around managed calls.
- Plugins install reusable runtime behavior from configuration.
- Events record runtime behavior in ATOF form.
- Subscribers and exporters consume events in-process or export them to ATIF, OpenTelemetry, OpenInference, or other backends.
Repository Structure
The repository layout separates the Rust runtime, language bindings,
documentation, integrations, and agent-facing skills.crates/ core/ # Rust core runtime crate, published as nemo-relay adaptive/ # Adaptive runtime primitives and plugin components python/ # PyO3 native extension for the Python package ffi/ # Raw C ABI layer used by downstream bindings such as Go node/ # NAPI Node.js binding and JavaScript/TypeScript entry points python/ nemo_relay/ # Python wrapper package: scopes, tools, LLM, middleware, typed helpers, plugins, adaptive helpers tests/ # Python tests go/ nemo_relay/ # Experimental Go CGo binding and tests fern/ # Fern documentation site scripts/ # Stable wrappers and helper scripts; build/test/docs entry points live in justfile skills/ # Published Codex/agent skills for NeMo Relay usage patternsPrerequisites
Insta...
Files:
python/tests/test_tools.pypython/tests/integrations/langgraph_tests/test_langgraph_integration.pypython/tests/integrations/deepagents_tests/test_deepagents_integration.pypython/tests/integrations/langchain_tests/test_middleware.py
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
python/tests/test_tools.pypython/tests/integrations/langgraph_tests/test_langgraph_integration.pypython/tests/integrations/deepagents_tests/test_deepagents_integration.pypython/tests/integrations/langchain_tests/test_middleware.py
🪛 Ruff (0.15.20)
python/tests/test_tools.py
[warning] 72-72: Lambda may be unnecessary; consider inlining inner function
Inline function call
(PLW0108)
🔇 Additional comments (1)
python/tests/integrations/langgraph_tests/test_langgraph_integration.py (1)
127-160: LGTM!
Signed-off-by: Will Killian <wkillian@nvidia.com>
willkill07
changed the title
willkill07
changed the title
|
/merge |
3 participants
Overview
Extends RELAY-443 coverage into the supported agent integrations. This is the second commit in a three-PR cumulative stack; all three PRs target upstream
mainas requested, so this PR temporarily includes the first commit until it merges.Details
srctrees.Validation:
just test-rust,just test-python,just test-node, anduv run pre-commit run --all-files.Breaking changes: none.
Where should the reviewer start?
Start with
crates/core/src/api/skill_load.rs, then review the coding-agent hook configurations and Python integration coverage.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
skill.loadmarks are emitted when structured skills are read via supported middleware and tooling.skill.loadevent payloads includeskill_nameplus attribution metadata (skill_load_source,tool_name) and that theparent_uuidlinks back to the associatedread_filestart scope.